-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V1.5 sync count patch #620
Conversation
WalkthroughThe pull request introduces updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/digit_data_model/CHANGELOG.md (1)
1-4
: Enhance changelog entry with more details.Consider expanding the changelog entry to include:
- The rationale behind making these parameters dynamic
- The benefits of this change
- Any migration steps if needed
## 1.0.4+1 * Bug Fix: - * Updated syncRetryCount,syncRetryInterval and errorPath to dynamic + * Made syncRetryCount, syncRetryInterval, and errorPath configurable at runtime + * This change allows for flexible retry policies and error handling based on environment needs + * Benefits include: + * Customizable retry attempts based on network conditions + * Configurable retry intervals for different environments + * Dynamic error handling endpoints for different deploymentspackages/digit_data_model/lib/data/repositories/oplog/oplog.dart (1)
115-116
: Consider adding validation for retry count.While the dynamic configuration is good, consider adding validation to ensure the retry count is a positive number.
+ if (DigitDataModelSingleton().syncDownRetryCount <= 0) { + throw ArgumentError('Retry count must be positive'); + } .syncDownRetryCountLessThan( DigitDataModelSingleton().syncDownRetryCount)packages/digit_data_model/lib/data/data_repository.dart (1)
Line range hint
367-385
: Document the update method's type change.The update method now accepts a broader EntityModel type instead of the generic type D. Consider adding documentation to explain this change and its implications.
+ /// Updates an existing entity. + /// Note: This method accepts any EntityModel type, not just type D. + /// This allows for more flexible entity updates but requires careful type checking. @override FutureOr<Response> update(EntityModel entity) async {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (4)
apps/health_campaign_field_worker_app/pubspec.lock
is excluded by!**/*.lock
,!**/*.lock
apps/health_campaign_field_worker_app/pubspec.yaml
is excluded by!**/*.yaml
packages/attendance_management/pubspec.lock
is excluded by!**/*.lock
,!**/*.lock
packages/digit_data_model/pubspec.yaml
is excluded by!**/*.yaml
📒 Files selected for processing (3)
packages/digit_data_model/CHANGELOG.md
(1 hunks)packages/digit_data_model/lib/data/data_repository.dart
(1 hunks)packages/digit_data_model/lib/data/repositories/oplog/oplog.dart
(4 hunks)
🔇 Additional comments (2)
packages/digit_data_model/lib/data/repositories/oplog/oplog.dart (2)
75-75
: LGTM! Dynamic retry count implementation.
The change from hardcoded value to DigitDataModelSingleton().syncDownRetryCount
improves configurability.
320-321
: 🛠️ Refactor suggestion
Validate retry interval to prevent excessive delays.
The retry interval multiplication could lead to very long delays for higher retry counts.
+ final maxDelay = 30; // Maximum delay in seconds
+ final calculatedDelay = DigitDataModelSingleton().retryTimeInterval *
+ oplogs.first.syncDownRetryCount;
+ final actualDelay = calculatedDelay > maxDelay ? maxDelay : calculatedDelay;
await Future.delayed(Duration(
- seconds: DigitDataModelSingleton().retryTimeInterval *
- oplogs.first.syncDownRetryCount,
+ seconds: actualDelay,
));
* updated hardcoded sync count, retry count and error api path * Published digit_data_model version update
* updated hardcoded sync count, retry count and error api path * Published digit_data_model version update Co-authored-by: naveen-egov <[email protected]>
Published digit_data_model version update with syncCount and retryCount fix